Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

docker_client: don't pull image and create container per tc command #251

Merged
merged 1 commit into from
Sep 7, 2023
Merged

docker_client: don't pull image and create container per tc command #251

merged 1 commit into from
Sep 7, 2023

Conversation

w-miller
Copy link
Contributor

Optimise the flow when using a separate container to run tc commands, by only pulling the container image once, creating a single container for the commands, and execing each command in that container.

@codecov-commenter
Copy link

codecov-commenter commented Aug 19, 2023 •
edited
Loading

Codecov Report

Patch coverage: 75.00% and project coverage change: +0.11% 🎉

Comparison is base (f3f7422) 40.11% compared to head (1d6d41c) 40.22%.
Report is 8 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #251      +/-   ##
==========================================
+ Coverage   40.11%   40.22%   +0.11%     
==========================================
  Files          37       37              
  Lines        1249     1218      -31     
==========================================
- Hits          501      490      -11     
+ Misses        748      728      -20
Files Changed Coverage Δ
pkg/container/docker_client.go 75.68% <75.00%> (+3.85%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alexei-led
Copy link
Owner

Hello @w-miller thank you for the PR.

But I would like to understand: what problem are you trying to solve with this PR? Is there any performance or stability issue with the current flow? Why do you think reusing the same tc container is a better option, than creating a new instance for every command? Did you measure the performance impact of this change?

@w-miller
Copy link
Contributor Author

w-miller commented Aug 21, 2023 •
edited
Loading

@alexei-led thanks for your response. Our pumba workflow involves using a separate container to run the tc commands (with --tc-image=gaiadocker/iproute2), and setting specific target IP addresses with the --target filter. We then typically use a long-ish duration for the pumba netem invocation, and terminate the container when we want to undo the rules. The motivation for this pull request was to reduce the time between invoking pumba, and the netem tc rules being successfully applied to the target container.

Explanation of the specific change in this patch: before the patch, pumba was attempting to do the following for every tc command it executes:

  • pull the image specified in --tc-image. Assuming that you already have the image present, this adds the overheads of making a network call to your container registry, and validating that the local image is still up to date.
  • create a new container to run the single tc command, execute the command, then stop and remove the container again.

It's probably worth mentioning that if you don't specify any IP or port filters, only a single tc command will be run, meaning this patch will behave identically to master. However, as soon as you add a single IP filter (for example), 5 tc commands are run, and this number further increases if you have additional IP or port filters.

I've run a benchmark locally with and without this patch to calculate the time between starting pumba and the netem tc rules being successfully applied. I compared these results between the code with and without the patch in this PR. N.B. in order to measure this timespan, I also needed to cherry-pick the patch in #250 to both of the pumba builds I tested, since the time at which the log message netem command started appears is the time at which the tc rules have all been applied, and there was no existing message logged at this point. I created a dummy container to be used as the target for the tc rules - a trivial container from the ubuntu image with the entrypoint sleep infinity. This container had name funny_napier. I used the following command, replacing pumba_image with a docker image built from each of the two versions of the code. I ran each variant 10 times
docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it <pumba_image> --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'

The full results are below, but the outcome was that the average time between pumba invocation and netem command started being logged was:

  • 10.86 seconds for the code without this patch
  • 3.24 seconds for the code with this patch

This result mirrors the speedup of around 3x that we saw in our testing workflows that use pumba after applying this patch.

Results without this PR 
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-only" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
9.808733 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:46:06Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-only" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
13.863111 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:46:28Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-only" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
13.772397 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:46:50Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-only" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
9.146954 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:47:08Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-only" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
8.720739 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:47:26Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-only" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
9.881483 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:47:45Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-only" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
9.600222 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:49:39Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-only" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
9.046138 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:50:51Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-only" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
10.480544 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:51:10Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-only" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
14.305070 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:51:53Z"}
Results with this PR 
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-and-251" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
2.971421 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:36:29Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-and-251" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
3.452594 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:42:14Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-and-251" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
3.060399 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:42:45Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-and-251" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
2.866957 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:43:03Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-and-251" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
3.174090 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:43:10Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-and-251" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
4.372453 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:43:20Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-and-251" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
2.992136 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:43:29Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-and-251" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
3.129593 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:43:40Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-and-251" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
3.374770 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:43:47Z"}
(base) will@will-ubuntu:~/proj/pumba$ docker run -t -v /var/run/docker.sock:/var/run/docker.sock -it "pumba-with-pr-250-and-251" --json --log-level=debug netem --duration=60s --tc-image=gaiadocker/iproute2 --target=192.168.0.22 loss --percent 10 funny_napier  | grep --line-buffered 'netem command started' | head -n1 | ts -s '%.s'
2.971335 {"dports":null,"duration":60000000000,"id":"ed177919460ec1848d1fce918310971498cc7c76a19ba223553fc33e09a0f666","iface":"eth0","ips":[{"IP":"192.168.0.22","Mask":"/////w=="}],"level":"debug","msg":"netem command started","name":"/funny_napier","netem":["loss","10.00"],"pull":true,"sports":null,"tc-image":"gaiadocker/iproute2","time":"2023-08-21T09:44:13Z"}

@w-miller w-miller force-pushed the will.miller/multiple-execs-per-container-2 branch from 8a6cac5 to 1d6d41c Compare August 21, 2023 10:32
@alexei-led alexei-led self-assigned this Sep 7, 2023
@alexei-led alexei-led self-requested a review September 7, 2023 07:21
@alexei-led alexei-led merged commit eeedb55 into alexei-led:master Sep 7, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexei-led alexei-led Awaiting requested review from alexei-led

Assignees

@alexei-led alexei-led

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@w-miller @codecov-commenter @alexei-led